Skip to content

tracing: add gpio tracing capabilities and sample #74199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Dec 4, 2024
Merged

tracing: add gpio tracing capabilities and sample #74199

merged 3 commits into from
Dec 4, 2024

Conversation

@alexanderlayTT
Copy link

@alexanderlayTT alexanderlayTT commented Jun 13, 2024 •
edited
Loading

If GPIO tracing is enabled, then the system will track input, output, interrupt attachment/removal and interrupt execution for GPIO pins.

@github-actions
Copy link

github-actions bot commented Jun 13, 2024

Hello @alexanderlayTT, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@alexanderlayTT alexanderlayTT force-pushed the gpio-tracing branch 8 times, most recently from 6184e72 to fe8af7c Compare June 18, 2024 19:03
@yangxuTT yangxuTT force-pushed the gpio-tracing branch 2 times, most recently from 2cbaac5 to 48cf5c4 Compare June 18, 2024 19:18
@alexanderlayTT alexanderlayTT force-pushed the gpio-tracing branch 2 times, most recently from cccfec0 to b216d18 Compare June 18, 2024 20:03
@alexanderlayTT alexanderlayTT force-pushed the gpio-tracing branch 8 times, most recently from 7ef7d0f to 3b5c6c2 Compare June 19, 2024 17:50
@zephyrproject-rtos zephyrproject-rtos deleted a comment from zephyrbot Jun 19, 2024
@cfriedt cfriedt changed the title Gpio Tracing tracing: add gpio tracing capabilities and sample Jun 19, 2024
@alexanderlayTT alexanderlayTT force-pushed the gpio-tracing branch 2 times, most recently from 490b3d3 to 33f55a7 Compare June 19, 2024 21:27
@cfriedt
Copy link
Member

cfriedt commented Nov 20, 2024

@alexanderlayTT - there is still a doc build issue

WARNING: /home/runner/work/zephyr/zephyr/include/zephyr/tracing/tracing.h:2595: unable to resolve reference to 'list' for \ref command
WARNING: /home/runner/work/zephyr/zephyr/include/zephyr/tracing/tracing.h:2595: unable to resolve reference to 'list' for \ref command

You should be able to reproduce with this
https://docs.zephyrproject.org/latest/contribute/documentation/generation.html

cfriedt
cfriedt previously approved these changes Nov 27, 2024
View reviewed changes
@cfriedt
Copy link
Member

cfriedt commented Nov 27, 2024

Looks good 👍 Thanks for the attention to detail @alexanderlayTT

ycsin
ycsin reviewed Nov 27, 2024
View reviewed changes
subsys/tracing/user/tracing_user.c Outdated
Copy link
Member

@ycsin ycsin Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to make the CI not complain about these, especially when the function is empty and __weak?

Copy link
Member

@cfriedt cfriedt Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this?

/* clang-format off */
...
/* clang-format on */

I don't expect that this needs to be added in this PR, since it's following the same convention used by existing code. However, having compliance failures means that only certain project members with extra privileges can merge this PR.

@nashif - what are you thoughts?

This question was already raised in a previous review comment.

@cfriedt cfriedt dismissed stale reviews from nashif and bjarki-andreasen November 28, 2024 15:26

outdated / comments addressed.

@cfriedt
Copy link
Member

cfriedt commented Nov 28, 2024

ping @bjarki-andreasen for a re-review

bjarki-andreasen
bjarki-andreasen previously approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nice work!

nashif
nashif reviewed Nov 28, 2024
View reviewed changes
include/zephyr/drivers/gpio.h Outdated
Copy link
Member

@nashif nashif Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use the macros? for example:

SYS_PORT_TRACING_OBJ_FUNC_ENTER(gpio_pin_interrupt_configure, pin, flags)
?

Copy link
Author

@alexanderlayTT alexanderlayTT Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a change to address this - PTAL

@alayander
tracing: add gpio tracing support
a041bac 
If GPIO tracing is enabled, then the system will track
various GPIO pin events.

Signed-off-by: Alexander Lay <[email protected]>
Signed-off-by: Yang Xu <[email protected]>
@alayander
samples: tracing: update sample app for gpio tracing
532d6ac 
Implement a sample app to perform basic GPIO tracing.

Signed-off-by: Alexander Lay <[email protected]>
Signed-off-by: Yang Xu <[email protected]>
ycsin
ycsin approved these changes Dec 4, 2024
View reviewed changes
include/zephyr/drivers/gpio.h

static inline int z_impl_gpio_port_get_raw(const struct device *port,
gpio_port_value_t *value)
static inline int z_impl_gpio_port_get_raw(const struct device *port, gpio_port_value_t *value)
Copy link
Member

@ycsin ycsin Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

such formatting changes should be in a separate commit

@nashif nashif merged commit d356403 into zephyrproject-rtos:main Dec 4, 2024
23 of 24 checks passed
@github-actions
Copy link

github-actions bot commented Dec 4, 2024

Hi @alexanderlayTT!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nashif nashif nashif approved these changes

@cfriedt cfriedt cfriedt approved these changes

@ycsin ycsin ycsin approved these changes

@bjarki-andreasen bjarki-andreasen bjarki-andreasen approved these changes

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

@mnkp mnkp Awaiting requested review from mnkp

@teburd teburd Awaiting requested review from teburd

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@kartben kartben Awaiting requested review from kartben

Assignees

@nashif nashif

Labels

area: GPIO area: Samples Samples area: Tracing Tracing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@alexanderlayTT @cfriedt @kartben @nashif @ycsin @bjarki-andreasen @zephyrbot @alayander